We can even further avoid the []byte->string allocation by using
_, ok := addrMap[string(consPk.Bytes())]
if ok {
...
}

...

addrMap[string(consPk.Bytes())] = ...
which will mos definitely show an improvement in the hot path.

No, the []byte -> string trick is not applicable, because consPk.Bytes() is a method call, the compiler need to allocate temporary variable for that, which is roughly like:

tmp := consPk.Bytes()
strKey := string(tmp)

Here's the benchmark result (comparing with this PR):

name                            old time/op    new time/op    delta
ValidateGenesis10Validators-8     1.13µs ± 1%    1.09µs ± 0%  -3.46%  (p=0.008 n=5+5)
ValidateGenesis100Validators-8    10.1µs ± 0%    10.3µs ± 1%  +2.90%  (p=0.008 n=5+5)
ValidateGenesis400Validators-8    38.3µs ± 0%    40.5µs ± 0%  +5.76%  (p=0.008 n=5+5)

name                            old alloc/op   new alloc/op   delta
ValidateGenesis10Validators-8       667B ± 0%      667B ± 0%    ~     (all equal)
ValidateGenesis100Validators-8    6.22kB ± 0%    6.22kB ± 0%    ~     (p=0.159 n=5+5)
ValidateGenesis400Validators-8    24.4kB ± 0%    24.4kB ± 0%    ~     (p=0.810 n=5+5)

name                            old allocs/op  new allocs/op  delta
ValidateGenesis10Validators-8       13.0 ± 0%      13.0 ± 0%    ~     (all equal)
ValidateGenesis100Validators-8       105 ± 0%       105 ± 0%    ~     (all equal)
ValidateGenesis400Validators-8       408 ± 0%       408 ± 0%    ~     (all equal)

it actually run slower because we call the method 2 times, which will introduce 2 temp vars.

Great point about it being a method call! However at least with a single variable then directly doing the map []->string call should work unless the compiler is getting faulty but I don’t see how doing m[string(bKey)] will make things slower it should eliminate the problem.

@odeke-em Hmm, not sure I got what you mean. What make thing slower because we call consPk.Bytes() 2 times, which will introduce 2 temp vars.

With those temp vars, the m[string([]byte)] optimization is applied already, that's why you can see your suggestion and the PR have the same alloc in benchmark result.

Did you see my second suggestion? Essentially adapt accordingly so:

bKey := consPk.Bytes()
m[string(bKey)]
...
m[string(bKey)]

No way that's not going to not eliminate the 2 []byte->string conversions.

With those temp vars, the m[string([]byte)] optimization is applied already, that's why you can see your suggestion and the PR have the same alloc in benchmark result.

I don't believe so fully, you are doing
strKey := string(...)

Then using strKey around, we don't have to incur that conversion at all given that strKey is being used as the key instead of m[string(bKey)]

@odeke-em What you do:

bKey := consPk.Bytes()

then do string(bKey) in lookup, is already the same thing with what string(consPk.Bytes()) does. Because the compiler rewrite it to:

tmp := consPk.Bytes()
strKey := string(tmp)

the compiler can realize that tmp isn't modified elsewhere, so the string conversion make no allocation. Here's the benchmark result with your second suggestion:

name                            old time/op    new time/op    delta
ValidateGenesis10Validators-8     1.14µs ± 0%    1.09µs ± 0%   -4.12%  (p=0.008 n=5+5)
ValidateGenesis100Validators-8    10.1µs ± 0%    11.3µs ±15%  +12.02%  (p=0.008 n=5+5)
ValidateGenesis400Validators-8    38.4µs ± 0%    40.5µs ± 1%   +5.42%  (p=0.008 n=5+5)

name                            old alloc/op   new alloc/op   delta
ValidateGenesis10Validators-8       667B ± 0%      667B ± 0%     ~     (all equal)
ValidateGenesis100Validators-8    6.22kB ± 0%    6.22kB ± 0%     ~     (p=1.000 n=5+5)
ValidateGenesis400Validators-8    24.4kB ± 0%    24.4kB ± 0%     ~     (p=0.206 n=5+5)

name                            old allocs/op  new allocs/op  delta
ValidateGenesis10Validators-8       13.0 ± 0%      13.0 ± 0%     ~     (all equal)
ValidateGenesis100Validators-8       105 ± 0%       105 ± 0%     ~     (all equal)
ValidateGenesis400Validators-8       408 ± 0%       408 ± 0%     ~     (all equal)

Tell me then, what does the next m[strKey] down below amount to instead of m[string(bKey)] much down below where there is not an edit? Not a hill am willing to die on and later on can be revised but would be nice to examine all the code paths.